-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Navigating branches and syncing #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got half way through reviewing and need to switch on to something else, so thought I'd just send you what I've done so far.
Some broad level comments are:
- I think the use of h3s throughout is a bit overkill and is starting to feel a bit forced (like just repeating the first sentence that follows it in some places). Feel like it'd be simpler if we ditched most of the h3s. If a page is getting so long that it feels like it needs a h3, then maybe that page just needs splitting into multiple pages.
- I'd like to make sure we've got some sort of text for all images. I've got it as caption text at the moment, but might be that the ideal is if we can figure out how to make it proper alt-text, so that it's hidden but gets read by screen readers. If that's awkward to do, then go with caption text.
On your questions:
if I should put instructions about pulling changes in syncing to the remote as well or leave it where it is
- I've adapted the sandbox now so that there is a pull section, but it comes after a bit of an intro to GitHub, so can be left until that's been written in another branch
i added steps for making changes in the recording changes qmd based on a discussion i had with lauren. is that ok?
- Yep, think that makes sense, just the comment to make it into a callout box
i added instructions for git status but can remove if ott. same for adding origin "branch name" after push and pull. if you prefer for me to change it to just git push and git pull, i'm happy to change it.
- I think it's useful referencing git status. I'd be tempted to change the git push ... to be
git push --set-upstream <github_id>_main
, but highlight that you only need to set the upstream on the first push, after that you can just usegit push
.
are the red rectangles/squares in the images not helpful or helpful?
- Helpful I'd say
The branch name i used in the pycharm screenshots has R in it. is that ok?
- in the sandbox, I've just removed references to specific software in the branch names now. Might be worth just redoing the images quickly to match
…teps and h3 headings and adapted the text to suit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more ideas for this one. Sorry if this feels like shifting sands a little, just thinking through the consistency and convention of how we write this as we're going. Something I've shifted to is a very pernickety / potentially annoying thing is having the branch name convention of rmbielby/main
and mzateddfe/main
instead of using the underscore. It's probably a superfluous tweak, but I feel like it could just make things parse a little more clearly when people are working through it.
…ext to make the tasks clearer based on RB's feedback
…e as the other pages
i ended up changing <github_username>_main and <github_id>_main variations to <github_username>/main even in scripts that i wasn't working on. hope that's ok! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant, thanks for all those changes!
I added instructions for:
For each of them, i added instructions for gitbash, R and Pycharm.
Things I'd appreciate feedback on in particular:
Closes #10
Closes #11